Skip to content

[iOS] Fix issue that does not allow scrolling in WebView with PDF#21571

Closed
jsuarezruiz wants to merge 10 commits into
mainfrom
fix-18716
Closed

[iOS] Fix issue that does not allow scrolling in WebView with PDF#21571
jsuarezruiz wants to merge 10 commits into
mainfrom
fix-18716

Conversation

@jsuarezruiz
Copy link
Copy Markdown
Contributor

Description of Change

Fix issue that does not allow scrolling in WebView with PDF on iOS.

Initially, I started by checking the status of the WebView: if internal UIScrollView scrolling was enabled, if the assigned size was correct, etc. Everything is fine so let's go check the hierarchy. It was interesting that the problem does not appear without a Layout as a parent. Reviewing I realized that the problem comes from the changes applied in #17286 related to InputTransparent. Applied changes to keep the InputTransparent working and fix the issue.

Issues Fixed

Fixes #18716

@jsuarezruiz jsuarezruiz added t/bug Something isn't working platform/ios area-controls-webview WebView labels Apr 2, 2024
@jsuarezruiz jsuarezruiz requested a review from a team as a code owner April 2, 2024 11:46
@dotnet-policy-service dotnet-policy-service Bot added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label Apr 2, 2024
Comment thread src/Core/src/Platform/iOS/LayoutView.cs Outdated
}

if (!result.UserInteractionEnabled)
if (Descendants is not null && Descendants.Contains(result) && !result.UserInteractionEnabled)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, the HitTest result could be an element of the hierarchy, but we are really only interested in elements of the hierarchy that come from Handlers, that is, those native elements created from an abstraction and that inherit from View and can use the InputTransparent property.

In the case of the WebView, the first element was _UIRemoteView with UserInteractionEnabled equals to false. This blocked interaction with the WebView. With the changes, we only verify elements added as children of the Layout, in the example added in this PR, it would only be a single WKWebView element, our WebView that is not disabled nor does use InputTransparent.

<Grid>
<WebView
x:Name="WaitForStubControl"
Source="https://freetestdata.com/wp-content/uploads/2021/09/Free_Test_Data_1MB_PDF.pdf">
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to maybe add a local pdf that would make sure we don't hit any connection issues loading this

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we can maybe at least host the pdf in the maui repo since we cannot control what this pdf is/does in the future.

Comment thread src/Core/src/Platform/iOS/LayoutView.cs Outdated
Comment thread src/Core/src/Platform/iOS/LayoutView.cs Outdated
Copy link
Copy Markdown
Member

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just a few nit comments.

{
this.IgnoreIfPlatforms(new TestDevice[] { TestDevice.Android, TestDevice.Mac, TestDevice.Windows });

await Task.Delay(1000); // Wait WebView to load.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This this gets flakey, we can have the webview page loaded event update some label and we can wait for that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated waiting a Label text after navigated.

<Grid>
<WebView
x:Name="WaitForStubControl"
Source="https://freetestdata.com/wp-content/uploads/2021/09/Free_Test_Data_1MB_PDF.pdf">
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we can maybe at least host the pdf in the maui repo since we cannot control what this pdf is/does in the future.

[Category(UITestCategories.WebView)]
public async Task CanScrollWebView()
{
this.IgnoreIfPlatforms(new TestDevice[] { TestDevice.Android, TestDevice.Mac, TestDevice.Windows });
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be worth enabling for all platforms?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enabled on more platforms. Scrolling using gestures, so Android and iOS.

}

if (!result.UserInteractionEnabled)
if (Subviews.Contains(result) && !result.UserInteractionEnabled)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to rather do result.Superview == this instead of getting the subviews array and looping through them all?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I am thinking we can't just check the result, we need to find the parent that is the child of the layout. So I am thinking:

while (result.Superview is not null && result.Superview != this)
    result = result.Superview;
if (result.Superview == this && !result.UserInteractionEnabled)

If we have a secret scroll view in our webview, we still need to find that webview and read the property the user set in XAML.

@mattleibow
Copy link
Copy Markdown
Member

What happens when the dev sets the webview as input transparent? can you scroll? what does the hit test result become? I cannot recall if the user interaction value is "viral" and the hit test blocks hitting all children.

@PureWeen
Copy link
Copy Markdown
Member

/rebase

@jsuarezruiz
Copy link
Copy Markdown
Contributor Author

The failing Windows Device Test is not related with the PR changes
image

@jsuarezruiz
Copy link
Copy Markdown
Contributor Author

Alternative PR #21883

@PureWeen
Copy link
Copy Markdown
Member

Closing in favor of
#21883

@PureWeen PureWeen closed this Apr 26, 2024
@mattleibow mattleibow deleted the fix-18716 branch April 26, 2024 17:33
@Eilon Eilon removed the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label May 10, 2024
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-controls-webview WebView platform/ios t/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[regression/8.0.3] [iOS] [SDK 17] Touch events are not working on WebView when a PDF is displayed.

5 participants